Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid adding extra comma separator if File input is empty or null #284

Merged
merged 5 commits into from
Apr 11, 2018

Conversation

fractalwrench
Copy link
Contributor

Previously, if the file input was null or empty, an additional comma was added by the JSON serialiser as a separator, leading to invalid JSON, e.g. [{}, , {}]. This can lead to session/error payloads being dropped as invalid.

This fixes JSON serialisation so that a comma is not included in this case, e.g. [{}, {}]

This doesn't fix the case where a file is non-empty but isn't valid JSON, which will be addressed separately.

@fractalwrench fractalwrench requested a review from a team April 10, 2018 08:43
appease the checkstyle gods
@fractalwrench fractalwrench requested a review from Pezzah April 10, 2018 08:49
@@ -49,6 +49,10 @@ public void value(@Nullable Streamable streamable) throws IOException {
* Writes a File (its content) into the stream
*/
public void value(@NonNull File file) throws IOException {
if (file == null || file.length() <= 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need the null check if @NonNull is used above?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@NonNull doesn't enforce anything in Java unfortunately. The IDE would highlight any null parameters with a warning, but developers could choose to ignore this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, fair enough.

@fractalwrench fractalwrench requested a review from kattrali April 10, 2018 15:59
@fractalwrench fractalwrench merged commit e3c2163 into next Apr 11, 2018
@fractalwrench fractalwrench deleted the json-empty-file-case branch April 11, 2018 10:24
rich-bugsnag pushed a commit that referenced this pull request Sep 3, 2021
Avoid using jcenter() in Unity gradle scripts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants